Adding MinScore support to Linear Retriever #126238
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mridula-s109
added
>enhancement
auto-backport
|
Hi @mridula-s109, I've created a changelog YAML for you. |
kderusso
reviewed
Apr 8, 2025
| @@ -346,7 +359,22 @@ public Matches matches(LeafReaderContext context, int doc) throws IOException { | |||
|
|
|||
| @Override | |||
| public ScorerSupplier scorerSupplier(LeafReaderContext context) throws IOException { | |||
| return combinedWeight.scorerSupplier(context); | |||
| ScorerSupplier supplier = combinedWeight.scorerSupplier(context); | |||
| if (minScore != DEFAULT_MIN_SCORE) { | |||
There was a problem hiding this comment.
Is this correct? Min score != rather than <=?
There was a problem hiding this comment.
yes we are just checking here if the user has provided a value? ie is it different from the default
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for the min_score parameter to the linear retriever to filter out documents with low relevance scores. Key changes include updates to YML tests for various min_score scenarios, modifications to LinearRetrieverBuilder and associated test cases, and accompanying enhancements in RankDocsQueryBuilder and documentation.
Reviewed Changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| x-pack/plugin/rank-rrf/src/yamlRestTest/resources/rest-api-spec/test/linear/10_linear_retriever.yml | Added new test cases for min_score functionality including default, negative, and zero thresholds. |
| x-pack/plugin/rank-rrf/src/main/java/org/elasticsearch/xpack/rank/linear/LinearRetrieverBuilder.java | Updated builder to support min_score filtering and validation (throwing error for negative values). |
| x-pack/plugin/rank-rrf/src/test/java/org/elasticsearch/xpack/rank/linear/LinearRetrieverBuilderTests.java | Added tests to verify the new min_score filtering behavior. |
| server/* | Updated RankDocsQueryBuilder, RankDocsRetrieverBuilder, and related cluster tests to propagate and validate min_score. |
| docs/reference/elasticsearch/rest-apis/retrievers.md | Extended documentation to describe the new min_score parameter. |
| docs/changelog/126238.yaml | Changelog entry reflecting the enhancement. |
Files not reviewed (1)
- x-pack/plugin/rank-rrf/build.gradle: Language not supported
Comments suppressed due to low confidence (1)
docs/reference/elasticsearch/rest-apis/retrievers.md:296
- [nitpick] Clarify in the documentation that the default min_score is set to Float.MIN_VALUE (a very small positive number) to effectively disable filtering, ensuring that users understand 0 is a valid threshold.
`min_score` : (Optional, float)
...lugin/rank-rrf/src/main/java/org/elasticsearch/xpack/rank/linear/LinearRetrieverBuilder.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for the min_score parameter in the linear retriever, enabling the filtering of documents based on a minimum relevance score. Key changes include:
- Implementing min_score filtering and validation in the LinearRetrieverBuilder and propagated components.
- Updating various unit, parsing, and integration tests to validate the new min_score behavior.
- Documenting the changes in YAML tests and reference documentation.
Reviewed Changes
Copilot reviewed 14 out of 16 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| x-pack/plugin/rank-rrf/src/yamlRestTest/resources/rest-api-spec/test/linear/10_linear_retriever.yml | Updates YAML tests to include various min_score scenarios |
| x-pack/plugin/rank-rrf/src/test/java/org/elasticsearch/xpack/rank/linear/LinearRetrieverBuilderTests.java | Adds unit tests for min_score filtering behavior |
| x-pack/plugin/rank-rrf/src/test/java/org/elasticsearch/xpack/rank/linear/LinearRetrieverBuilderParsingTests.java | Updates parsing tests to propagate default min_score |
| x-pack/plugin/rank-rrf/src/main/java/org/elasticsearch/xpack/rank/linear/MinMaxScoreNormalizer.java | Adjusts normalization logic for scores, handling NaN values |
| x-pack/plugin/rank-rrf/src/main/java/org/elasticsearch/xpack/rank/linear/LinearRetrieverBuilder.java | Introduces min_score as part of the builder with validation and propagation |
| x-pack/plugin/rank-rrf/src/internalClusterTest/java/org/elasticsearch/xpack/rank/linear/LinearRetrieverIT.java | Enhances integration tests to verify behavior under multiple min_score conditions |
| server/src/test/java/org/elasticsearch/index/query/RankDocsQueryBuilderTests.java | Adds tests verifying serialization, parsing, and filtering based on min_score |
| server/src/main/java/org/elasticsearch/search/retriever/rankdoc/RankDocsQuery.java | Uses min_score when deciding final document scores |
| server/src/main/java/org/elasticsearch/search/retriever/RankDocsRetrieverBuilder.java | Propagates min_score to the search source builder |
| server/src/main/java/org/elasticsearch/index/query/RankDocsQueryBuilder.java | Updates builder to include min_score for filtering and serialization |
| server/src/main/java/org/elasticsearch/TransportVersions.java | Introduces a new transport version for min_score support |
| docs/reference/elasticsearch/rest-apis/retrievers.md | Documents the new min_score parameter and usage examples |
| docs/changelog/126238.yaml | Lists the change for adding MinScore support |
Files not reviewed (2)
- x-pack/plugin/rank-rrf/build.gradle: Language not supported
- x-pack/plugin/rank-rrf/src/test/java/org/elasticsearch/xpack/rank/linear/LinearRetrieverBuilderTests.java.new: Language not supported
Comments suppressed due to low confidence (1)
x-pack/plugin/rank-rrf/src/yamlRestTest/resources/rest-api-spec/test/linear/10_linear_retriever.yml:1219
- The expected error message regex in the test does not match the exception message thrown by LinearRetrieverBuilder (which indicates '[min_score] must be greater than or equal to 0'). Update the regex or exception message so that they are consistent.
catch: "/\[min_score\] must be greater than 0, was: -1\.0/"
d648579 to
d27b67b
Compare
mridula-s109
force-pushed
the
add_min_score_linear_retriever
branch
from
d27b67b to
13bce60
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR introduces support for the min_score parameter in the linear retriever. The min_score filter ensures that only documents with a relevance score above a specified threshold are retrieved, improving result quality and filtering out low-relevance matches.
Added min_score support in the linear retriever.
Updated the retrieval logic to apply the min_score threshold.
Based on an example from the Elasticsearch Guide (8.18), extended it to demonstrate min_score usage.